Plugins: no server; interface contravariant; no factory functions#6814
Merged
Plugins: no server; interface contravariant; no factory functions#6814
Conversation
✅ Deploy Preview for apollo-server-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit f5b4133:
|
b2611d5 to
5015fe0
Compare
Previously on `version-4`, we had made `ApolloServerPlugin<TContext>`
"invariant" in `TContext` via the new `in out` annotation. That's
because we had added a `server` field to `GraphQLRequestContext` and
`GraphQLServerContext`.
The issue here is that because you can invoke
`server.executeOperation(op, contextValue)`, then a
`ApolloServerPlugin<BaseContext>` was not assignable to
`ApolloServerPlugin<SomeSpecificContext>`, because the former was able
to call `requestContext.executeOperation(op, {})`, and a server whose
context has additional required fields could not load this plugin. This
essentially meant that you couldn't have a single object (not created by
a generic function or generic class) that was a plugin that works with
any server, even if it doesn't actually care about context values at
all.
When trying to dogfood AS4 it became clear that this was annoying. So
let's fix it by removing `server` from those objects and making
`ApolloServerPlugin` "contravariant" instead. This means that plugins
can only *receive* `contextValue`, not *send* it. And so
`ApolloServerPlugin<BaseContext>` (a plugin that doesn't bother to read
any fields from `contextValue`) can be assigned to
`ApolloServerPlugin<SpecificContext>` (a plugin that is permitted to
read specific fields on `contextValue`, though it may choose not to).
After removing `server`, restore `logger` (now `readonly`) and `cache`
to `GraphQLRequestContext` and `GraphQLServerContext` (`cache` is
actually new to `GraphQLServerContext` in AS4). They had previously been
removed for redundancy.
This un-does the fix to #6264. If your plugin needs access to the
`server` object, you can just pass it to your plugin's
constructor/factory yourself. This wasn't an option in AS3 because there
was no `server.addPlugin` method... or actually, you could do it with
the plugin factory feature.
Note that we do leave `ApolloServer<TContext>` as invariant in
`TContext`, for the reasons described in a comment above the class.
While we're at it, remove the unnecessary ability to specify plugins as
factory functions. As far as we know, the main use case for this feature
is to refer to the `ApolloServer` itself in the expression creating the
plugin, but the new `addPlugin` method lets you do the same thing.
Reducing the number of ways to specify constructor options helps keep
type errors simpler.
5015fe0 to
f5b4133
Compare
Member
Author
|
Going to merge this without full review so that I can make another alpha and move forward, but happy to make big changes or even revert after feedback. |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously on
version-4, we had madeApolloServerPlugin<TContext>"invariant" in
TContextvia the newin outannotation. That'sbecause we had added a
serverfield toGraphQLRequestContextandGraphQLServerContext.The issue here is that because you can invoke
server.executeOperation(op, contextValue), then aApolloServerPlugin<BaseContext>was not assignable toApolloServerPlugin<SomeSpecificContext>, because the former was ableto call
requestContext.executeOperation(op, {}), and a server whosecontext has additional required fields could not load this plugin. This
essentially meant that you couldn't have a single object (not created by
a generic function or generic class) that was a plugin that works with
any server, even if it doesn't actually care about context values at
all.
When trying to dogfood AS4 it became clear that this was annoying. So
let's fix it by removing
serverfrom those objects and makingApolloServerPlugin"contravariant" instead. This means that pluginscan only receive
contextValue, not send it. And soApolloServerPlugin<BaseContext>(a plugin that doesn't bother to readany fields from
contextValue) can be assigned toApolloServerPlugin<SpecificContext>(a plugin that is permitted toread specific fields on
contextValue, though it may choose not to).After removing
server, restorelogger(nowreadonly) andcacheto
GraphQLRequestContextandGraphQLServerContext(cacheisactually new to
GraphQLServerContextin AS4). They had previously beenremoved for redundancy.
This un-does the fix to #6264. If your plugin needs access to the
serverobject, you can just pass it to your plugin'sconstructor/factory yourself. This wasn't an option in AS3 because there
was no
server.addPluginmethod... or actually, you could do it withthe plugin factory feature.
Note that we do leave
ApolloServer<TContext>as invariant inTContext, for the reasons described in a comment above the class.While we're at it, remove the unnecessary ability to specify plugins as
factory functions. As far as we know, the main use case for this feature
is to refer to the
ApolloServeritself in the expression creating theplugin, but the new
addPluginmethod lets you do the same thing.Reducing the number of ways to specify constructor options helps keep
type errors simpler.